Support Frequency schema names #46

wesbiggs · 2023-12-09T16:57:40Z

Deploy now uses dsnp.* schema names and new versions of add/propose schema extrinsics.
Adds dsnp.getSchemaId() and utility types/methods for registering non-standard mappings (see README.md).
New find.ts CLI script will do a forward lookup of DSNP schema names on chain.

cf. frequency-chain/frequency#1784

Updated to use @frequency-chain/[email protected].

aramikm

overall looks good, some minor comments!

dsnp/index.ts

cli/find.ts

dsnp/index.ts

JoeCap08055 · 2023-12-11T21:20:37Z

I'm assuming this shouldn't be merged until the next release of PolkadotJS, because the new extrinsics aren't accessible/published in the JS API until then... or at least, that's what I understand from this:

Attempting to create tombstone schema.
TypeError: api.tx.schemas.createSchemaViaGovernanceV2 is not a function
    at createSchemas (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:106:39)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async deploy (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:41:21)
    at async main (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/index.ts:3:5)

wesbiggs · 2023-12-11T21:38:38Z

I'm assuming this shouldn't be merged until the next release of PolkadotJS, because the new extrinsics aren't accessible/published in the JS API until then... or at least, that's what I understand from this:
Attempting to create tombstone schema.
TypeError: api.tx.schemas.createSchemaViaGovernanceV2 is not a function
    at createSchemas (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:106:39)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async deploy (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:41:21)
    at async main (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/index.ts:3:5)

It doesn't work on the current release of the Docker image—is that what you mean? I tested against a make start version running locally.

JoeCap08055 · 2023-12-11T21:53:29Z

I'm assuming this shouldn't be merged until the next release of PolkadotJS, because the new extrinsics aren't accessible/published in the JS API until then... or at least, that's what I understand from this:
Attempting to create tombstone schema.
TypeError: api.tx.schemas.createSchemaViaGovernanceV2 is not a function
    at createSchemas (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:106:39)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async deploy (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/deploy.ts:41:21)
    at async main (file:///Users/joecaputo/src/LibertyDSNP/schemas/cli/index.ts:3:5)
It doesn't work on the current release of the Docker image—is that what you mean? I tested against a make start version running locally.

Ah, actually, I think I misspoke. I believe the PolkadotJS should work fine once the updated chain is deployed (I think it's only the RPC calls that depend on @polkadotjs packages being updated; extrinsics should be fine (@aramikm am I correct?). But this still won't work until the next Frequency release (1.9.1? 1.10.0?) is deployed to testnet & mainnet.

wesbiggs · 2023-12-11T21:58:13Z

But this still won't work until the next Frequency release (1.9.1? 1.10.0?) is deployed to testnet & mainnet.

Yes, correct, it relies on the new extrinsics/RPC endpoints. We can wait for that before releasing this. (And yeah, let me know what version to watch for.)

aramikm

Looks great!

shannonwells

This may be operator error, but I'm having issues including this branch in a project (@wilwade 's magic-test repo) and using it. It works fine with main branch of schemas.

in this repo + branch: npm link
in test project, npm i @dsnp/frequency-schemas
in test project, run npm link @dsnp/frequency-schemas and verify it's now symlinked to the repo inside of node_modules/@DSNP
Do the import and add the code shown:

import { dsnp } from "frequency-schemas";

const main = async () => {
  console.log(dsnp.getSchema("broadcast"));
}

main().catch(console.error).finally(process.exit);

Then run npm run main to execute the code. It's trying to find parquetjs inside of @dsnp/frequency-schemas:

  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/shannonwells/.asdf/installs/nodejs/18.15.0/.npm/lib/node_modules/@dsnp/frequency-schemas/cjs/parquet.js',
    '/Users/shannonwells/.asdf/installs/nodejs/18.15.0/.npm/lib/node_modules/@dsnp/frequency-schemas/cjs/index.js',
    '/Users/shannonwells/github/magic-test/main.ts'
  ]

shannonwells · 2023-12-12T22:24:57Z

README.md

+### Get Schema Id from Chain
+
+```typescript
+import { dsnp } from "frequency-schemas";


blocking: need to prefix with @DSNP

Suggested change

import { dsnp } from "frequency-schemas";

import { dsnp } from "@dsnp/frequency-schemas";

Committed that change. Was that the issue?

@wesbiggs Are you sure you committed the change here? I don't see it.

Good catch, fixed.

shannonwells · 2023-12-12T22:35:13Z

package.json

@@ -7,6 +7,7 @@
    "test": "jest",
    "deploy": "node --loader ts-node/esm cli/index.ts",
    "read": "node --loader ts-node/esm cli/read.ts",
+    "find": "node --loader ts-node/esm cli/find.ts",


Find works against a local chain.

* Deploy now uses `dsnp.*` schema names and new versions of add/propose schema extrinsics.. * Adds `dsnp.getSchemaId()` and utility types/methods for registering non-standard mappings. * New find.ts CLI script will do a forward lookup of DSNP schema names on chain.

enddynayn · 2024-01-04T15:25:21Z

cli/deploy.ts

-              console.log("SUCCESS: " + schemaName + " schema created with id of " + val);
-              resolve({ schemaName, id: Number(val.toHuman()) });
-            } else resolve({ schemaName });
+              const id = evt?.data[1];


maybe not a big deal for your purposes but do you need to close the subscription?

I think so, for correctness. Probably doesn't matter too much since it's a run-and-done CLI program, but I wouldn't be surprised if there are some error logs that come out when the program is done that would be cleaned up with an unsub() call. I'll pull this branch and see...

I see no errors when I run; like I said, probably not a critical issue since this is CLI run-and-done. Fix if you like; non-blocking.

I don't think we're actually subscribed here -- if the event is not immediately in the block or finalized, it looks like we never resolve or reject the Promise. Obviously this seems to work fine in an ideal environment (especially one with instant sealing) but I don't think it handles edge cases or busy chains very well.

I'd prefer to make that a separate issue though as it's a pre-existing condition, and maybe someone slightly more familiar with how to subscribe and poll could add this or give me some pointers.

Added as #49

cli/deploy.ts

wilwade

🚢

Ran locally
Tested with docker

shannonwells

Looks good, thanks

wesbiggs requested review from wilwade, aramikm and JoeCap08055 December 9, 2023 16:57

wesbiggs self-assigned this Dec 9, 2023

wesbiggs mentioned this pull request Dec 9, 2023

Document schema IDs for dsnp/instant-seal-node-with-deployed-schemas #44

Closed

aramikm reviewed Dec 11, 2023

View reviewed changes

dsnp/index.ts Show resolved Hide resolved

cli/find.ts Outdated Show resolved Hide resolved

dsnp/index.ts Outdated Show resolved Hide resolved

dsnp/index.ts Outdated Show resolved Hide resolved

dsnp/index.ts Show resolved Hide resolved

aramikm approved these changes Dec 12, 2023

View reviewed changes

JoeCap08055 approved these changes Dec 12, 2023

View reviewed changes

shannonwells requested changes Dec 12, 2023

View reviewed changes

Wes Biggs added 3 commits January 3, 2024 15:26

Address PR comments

a6da933

Correct explanation of "default" mapping

40db31e

wesbiggs force-pushed the feature/schema-names branch from 11b9e21 to 40db31e Compare January 3, 2024 21:27

Update all dependency versions, but especially api-augment to 1.10.0.

2d5b321

wesbiggs enabled auto-merge (squash) January 3, 2024 21:33

Fix reference to this package.

638602c

enddynayn reviewed Jan 4, 2024

View reviewed changes

JoeCap08055 reviewed Jan 4, 2024

View reviewed changes

cli/deploy.ts Outdated Show resolved Hide resolved

wilwade requested a review from shannonwells January 4, 2024 18:14

Get better at handling rejection

df28127

wilwade approved these changes Jan 5, 2024

View reviewed changes

shannonwells approved these changes Jan 9, 2024

View reviewed changes

wesbiggs merged commit 0bcd42f into main Jan 9, 2024
1 check passed

wesbiggs deleted the feature/schema-names branch January 9, 2024 01:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Frequency schema names #46

Support Frequency schema names #46

wesbiggs commented Dec 9, 2023 •

edited

Loading

aramikm left a comment

JoeCap08055 commented Dec 11, 2023

wesbiggs commented Dec 11, 2023

JoeCap08055 commented Dec 11, 2023

wesbiggs commented Dec 11, 2023

aramikm left a comment

shannonwells left a comment •

edited

Loading

shannonwells Dec 12, 2023

wesbiggs Dec 13, 2023

wilwade Jan 4, 2024

wesbiggs Jan 4, 2024

shannonwells Dec 12, 2023

enddynayn Jan 4, 2024 •

edited

Loading

JoeCap08055 Jan 4, 2024

JoeCap08055 Jan 4, 2024

wesbiggs Jan 4, 2024

wesbiggs Jan 5, 2024

wilwade left a comment

shannonwells left a comment

	import { dsnp } from "frequency-schemas";
	import { dsnp } from "@dsnp/frequency-schemas";

Support Frequency schema names #46

Support Frequency schema names #46

Conversation

wesbiggs commented Dec 9, 2023 • edited Loading

aramikm left a comment

Choose a reason for hiding this comment

JoeCap08055 commented Dec 11, 2023

wesbiggs commented Dec 11, 2023

JoeCap08055 commented Dec 11, 2023

wesbiggs commented Dec 11, 2023

aramikm left a comment

Choose a reason for hiding this comment

shannonwells left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enddynayn Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilwade left a comment

Choose a reason for hiding this comment

shannonwells left a comment

Choose a reason for hiding this comment

wesbiggs commented Dec 9, 2023 •

edited

Loading

shannonwells left a comment •

edited

Loading

enddynayn Jan 4, 2024 •

edited

Loading